feat: handle theme with localStorage#456
Conversation
✅ Deploy Preview for tanstack ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
src/components/ThemeProvider.tsx
Outdated
| type ThemeMode = z.infer<typeof themeModeSchema> | ||
| type ResolvedTheme = z.infer<typeof resolvedThemeSchema> | ||
|
|
||
| function getStoredThemeMode(): ThemeMode { |
There was a problem hiding this comment.
we could make this an isomorphic function using createIsomorphicFn
There was a problem hiding this comment.
(this applies to the other functions as well)
There was a problem hiding this comment.
What is the advantage?
| const storedTheme = localStorage.getItem('theme') || 'auto' | ||
|
|
||
| if (storedTheme === 'auto') { | ||
| const autoTheme = window.matchMedia('(prefers-color-scheme: dark)') |
There was a problem hiding this comment.
do we have a subscription to theme change on the client? e.g. if i switch from dark to light mode on the client in my browser or OS settings, does this update the theme here as well?
There was a problem hiding this comment.
Yes, it's the setupPreferredListener function setting it up. While on auto if you change the theme for example on your OS setting it immediately updates on tanstack.com
| document.documentElement.classList.add(storedTheme) | ||
| } | ||
| } catch (e) { | ||
| const autoTheme = window.matchMedia('(prefers-color-scheme: dark)') |
There was a problem hiding this comment.
why do we call this again? when does the above throw?
There was a problem hiding this comment.
This is just in case accessing localStorage fails
| export function ThemeProvider({ children }: ThemeProviderProps) { | ||
| const [themeMode, setThemeMode] = useState<ThemeMode>(getStoredThemeMode) | ||
|
|
||
| useEffect(() => { |
There was a problem hiding this comment.
do we really need that effect? cant we just execute it whenever setTheme is called?
There was a problem hiding this comment.
I can move updateThemeClass inside setTheme but I'd keep the useEffect to un/subscribe to the system listener
Balastrong
force-pushed
the
feat/theme-localstorage
branch
from
cef7935 to
e6ffe3c
Compare
3 participants
As discussed on Discord I removed the cookie logic in favor of localStorage to handle the theme (light, dark and auto)
It previously relied on zusatand, I replaced it with a react context in a separate commit, you can see both versions in this PR.
I think we should keep it simple and stay with the context but if we want to use zustand as it's already installed I can revert the second commit.